Skip to content

Created samples for TokenizeIntoWords and RemoveStopWords APIs. - #3156

Merged
zeahmed merged 8 commits into
dotnet:masterfrom
zeahmed:TokenizeIntoWords
Apr 2, 2019
Merged

Created samples for TokenizeIntoWords and RemoveStopWords APIs.#3156
zeahmed merged 8 commits into
dotnet:masterfrom
zeahmed:TokenizeIntoWords

Conversation

@zeahmed

@zeahmed zeahmed commented Apr 1, 2019

Copy link
Copy Markdown
Contributor

Related to #1209.

@codecov

codecov Bot commented Apr 1, 2019

Copy link
Copy Markdown

Codecov Report

Merging #3156 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3156      +/-   ##
==========================================
+ Coverage   72.53%   72.53%   +<.01%     
==========================================
  Files         808      807       -1     
  Lines      144775   144774       -1     
  Branches    16209    16208       -1     
==========================================
+ Hits       105012   105018       +6     
+ Misses      35348    35341       -7     
  Partials     4415     4415
Flag Coverage Δ
#Debug 72.53% <ø> (ø) ⬆️
#production 68.12% <ø> (ø) ⬆️
#test 88.82% <ø> (ø) ⬆️
Impacted Files Coverage Δ
src/Microsoft.ML.Transforms/Text/TextCatalog.cs 41.66% <ø> (ø) ⬆️
...ML.Transforms/Text/StopWordsRemovingTransformer.cs 86.1% <0%> (-0.16%) ⬇️
src/Microsoft.ML.Data/Transforms/Normalizer.cs 86.03% <0%> (ø) ⬆️
...icrosoft.ML.Functional.Tests/DataTransformation.cs 100% <0%> (ø) ⬆️
...s/Api/CookbookSamples/CookbookSamplesDynamicApi.cs 93.49% <0%> (ø) ⬆️
...s/Scenarios/Api/CookbookSamples/CookbookSamples.cs 99.49% <0%> (ø) ⬆️
test/Microsoft.ML.Functional.Tests/Training.cs 100% <0%> (ø) ⬆️
...est/Microsoft.ML.Tests/FeatureContributionTests.cs 98.55% <0%> (ø) ⬆️
...oft.ML.Experimental/TransformsCatalogExtensions.cs
...c/Microsoft.ML.FastTree/Utils/ThreadTaskManager.cs 100% <0%> (+20.51%) ⬆️
... and 1 more

@sfilipi
sfilipi requested a review from natke April 1, 2019 21:09
var mlContext = new MLContext();

// Create an empty data sample list. The 'RemoveDefaultStopWords' does not require training data as
// the estimator ('StopWordsRemovingEstimator') created by 'RemoveDefaultStopWords' API is not a trainable estimator.

@Ivanidzo4ka Ivanidzo4ka Apr 1, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StopWordsRemovingEstimator [](start = 31, length = 26)

any reason why you prefer this format other than <see cref="StopWordsRemovingEstimator"> ? #Pending

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt if it works in normal comments. works in xml comments.


In reply to: 271080226 [](ancestors = 271080226)

// as well as the source of randomness.
var mlContext = new MLContext();

// Create an empty data sample list. The 'RemoveDefaultStopWords' does not require training data as

@rogancarr rogancarr Apr 1, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data sample list. [](start = 30, length = 18)

Create an empty list as the dataset. #Resolved

var emptyDataView = mlContext.Data.LoadFromEnumerable(emptySamples);

// A pipeline for converting text into vector of words.
var textPipeline = mlContext.Transforms.Text.TokenizeIntoWords("Words", "Text", separators: new[] { ' ' });

@rogancarr rogancarr Apr 1, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

separators: new[] { ' ' } [](start = 92, length = 25)

Can you add details about what to expect here with default values? One thing we may want to mention is removing any whitespace. #Resolved

@rogancarr rogancarr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with comments.

@Ivanidzo4ka Ivanidzo4ka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

/// <![CDATA[
/// [!code-csharp[FastTree](~/../docs/samples/docs/samples/Microsoft.ML.Samples/Dynamic/StopWordRemoverTransform.cs)]
/// ]]></format>
/// [!code-csharp[RemoveStopWords](~/../docs/samples/docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Text/RemoveDefaultStopWords.cs)]

@Ivanidzo4ka Ivanidzo4ka Apr 2, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RemoveStopWords [](start = 26, length = 15)

RemoveDefaultStopWords
not sure is it important or not #Resolved

// A pipeline for removing stop words from input text/string.
// The pipeline first tokenizes text into words then removes stop words.
var textPipeline = mlContext.Transforms.Text.TokenizeIntoWords("Words", "Text")
.Append(mlContext.Transforms.Text.RemoveStopWords("WordsWithoutStopWords", "Words", stopwords: new[] { "a", "the", "from", "by" }));

@Ivanidzo4ka Ivanidzo4ka Apr 2, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from [](start = 132, length = 4)

I would throw few words regarding casing.
imaging you have string "Something CoOl here" and you have stop word remover with "cool", would it remove it? Would it preserve it?
It's a mystery now, but you can show how it works in this sample. #Resolved

var emptyDataView = mlContext.Data.LoadFromEnumerable(emptySamples);

// A pipeline for removing stop words from input text/string.
// The pipeline first tokenizes text into words then removes stop words.

@Ivanidzo4ka Ivanidzo4ka Apr 2, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removes [](start = 65, length = 7)

shall we add link to list of stop words?
https://github.com/dotnet/machinelearning/blob/master/src/Microsoft.ML.Transforms/Text/StopWords/English.txt this one is for english. #Resolved

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also if you can modify string to include "tHe" or something like that and show it was removed (because we compare by ignoring casing (I hope so) would be nice.


In reply to: 271477611 [](ancestors = 271477611)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the link should go into the documentation instead of sample.


In reply to: 271478137 [](ancestors = 271478137,271477611)

@Ivanidzo4ka

Copy link
Copy Markdown
Contributor
        var data = new TextData() { Text = "This is a great product. I would like to buy it again."  };

It's me, your friendly casing neighbor.
In my example you can saw what xbox got treated even if we specify XBOX in file.


Refers to: docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Text/ApplyCustomWordEmbedding.cs:54 in 1bc241d. [](commit_id = 1bc241d, deletion_comment = False)

@zeahmed
zeahmed merged commit 950f133 into dotnet:master Apr 2, 2019
@zeahmed

zeahmed commented Apr 2, 2019

Copy link
Copy Markdown
Contributor Author

Thanks!

zeahmed added a commit to zeahmed/machinelearning that referenced this pull request Apr 8, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Mar 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants